Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a base-only provider-graph validation edge case where inspecting a base image by config ID can yield an internally inconsistent identity (Docker-reported config-ID descriptor vs the originally resolved registry descriptor). The change restores the resolved base descriptor/image identity so downstream validation and application runtime-layer builds remain consistent when no provider layers are present.
Changes:
- In base-only graphs, restore the resolved base
Descriptorand base prefixImageafter inspection to preserve the registry-resolved identity. - Relax inspected-candidate identity validation to accept
Image.Digestmatching either the descriptor’s config digest or (when present) manifest digest. - Update the base-only graph validation test to simulate Docker’s config-ID inspection behavior and assert the resolved descriptor is preserved; add a changelog fragment.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/dockerdeploy/provider_graph_validation.go | Restores the resolved base descriptor/image identity for base-only graphs after inspection-by-config-ID. |
| internal/dockerdeploy/provider_graph_validation_test.go | Updates the base-only test to reflect Docker’s config-ID descriptor reporting and asserts the resolved descriptor is preserved. |
| internal/dockerdeploy/materialization_layer.go | Allows inspected identity validation to accept manifest digest matches when a descriptor carries a manifest digest. |
| .changes/unreleased/+base-only-direct-install.yaml | Adds a release-note fragment documenting the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e9fe69f to
af3263b
Compare
cc8bf62 to
d2ea9e5
Compare
ba9173d to
640371f
Compare
db242f2 to
90b9c39
Compare
71e2370 to
fa3306d
Compare
d5d3a5f to
fc8b283
Compare
2a5e37c to
59ee228
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d717aff70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // identity so the inspected candidate remains internally consistent and | ||
| // the application runtime layer connects to the graph's base prefix. | ||
| candidate.Descriptor = base | ||
| candidate.Image = graph.PrefixImages[0] |
There was a problem hiding this comment.
For a registry-backed base-only graph, this makes RuntimeLayer.Upstream.Digest the manifest digest. If installation changes the local account—most notably a Linux system-scope install from the temporary staged build—buildInstalledRuntimeIdentityWithV1 reinspects that upstream by config ID; InspectBuiltImageCandidate reports Digest == ConfigDigest, so the strict comparison in install_runtime_identity.go:58-67 rejects the unchanged image before rebuilding the runtime identity. Normalize that reinspection back to the locked base identity and descriptor so these installs do not fail.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Stack created with Sapling. Best reviewed with ReviewStack.